Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: contentFitScreen and contentFitContainer Display Modes #2272

Merged
merged 6 commits into from
May 13, 2022

Conversation

Joshua-Beatty
Copy link
Contributor

@Joshua-Beatty Joshua-Beatty commented Mar 9, 2022

===:clipboard: PR Checklist :clipboard:===

  • 📌 issue exists in github for these changes
  • 🔬 existing tests still pass
  • 🙈 code conforms to the style guide
  • 📐 new tests written and passing / old tests updated with new scenario(s)
  • 📄 changelog entry added (or not needed)

==================

Closes #2271

Changes:

  • Created Display Mode ContentFitScreen
  • Created Display Mode ContentFitContainer

@Joshua-Beatty
Copy link
Contributor Author

Currently unable to run tests on my machine, having issues with ChromeHeadless.

Known issue:
Currently on Android(Through Capacitor and Cordova) and I believe iOS(through capacitor, haven't tested Cordova) I get the following error:

2022-03-08 23:06:22.648 2051-2051/me.joshbeatty.speedsquared W/Capacitor/Console: File: http://localhost/ - Line 0 - Msg: [.WebGL-0xebe0c6f0]RENDER WARNING: texture bound to texture unit 15 is not renderable. It might be non-power-of-2 or have incompatible texture filtering (maybe)?

And no sprites render. Example repository
Note: this is a very poor example repository with a mish mash of build tools, and this fork of Excalibur being referenced in the typescript relatively, outside of the repo. I can work on getting a more bare-bones and clean example repository.

@Joshua-Beatty Joshua-Beatty changed the title Creating contentFitScreen and contentFitContainer Display Modes feat: contentFitScreen and contentFitContainer Display Modes Mar 9, 2022
@eonarheim
Copy link
Member

@Joshua-Beatty I had a similar issue in Android Capacitor when running in the simulator VM, not sure why this happens yet. It does work when deployed to my mobile device.

As a workaround in the simulator I had to explicitly set the graphics support (auto didn't seem to work?) then restart android studio. Let me know if this works for you

image

@eonarheim
Copy link
Member

eonarheim commented Mar 10, 2022

Currently unable to run tests on my machine, having issues with ChromeHeadless.

I'm interested in your test issues, definitely want to fix this if it's an excalibur problem.

@Joshua-Beatty
Copy link
Contributor Author

After doing some further testing, I found the issue to be with the viewport size. By adding suppressHiDPIScaling: true it ran in the emulator properly. My current implementation sets the viewport to be the size of the screen, then sets the resolution to the correct scale. Is their any downside to setting the viewport to be equal to the resolution?

@Joshua-Beatty
Copy link
Contributor Author

Looks like just setting the view port to be the resolution didn't fix it. It seems the fix is to just keep the resolution low by setting a small content size, or enabling suppressHiDPIScaling. What are the downsides of enabling suppressHiDPIScaling? The docs don't cover it too well unless i missed something. Is it just performance?

@eonarheim
Copy link
Member

@Joshua-Beatty Apologies on the delay, the HiDPI Scaling scales the internal canvas resolution to avoid an interesting quirk of HTML canvas rendering that will cause graphics to look blurry on HiDPI devices (the reason is images are spread across at least twice as many physical pixels). I'll definitely make a note to improve the documentation around this area. Disabling the HiDPI scaling may theoretically improve performance because the browser will have to drive less pixels.

I suspect the reason it didn't work in the emulator is the scaling cased the render target texture to be bigger than allowed on the mobile device (usually 4k pixels in any dimension will cause texture loading to fail on mobile). Out of curiosity what resolution do you have set?

I had some time this weekend to poke around some of the code after looking at the Solar2D zoomEven scaling you shared. I think I have something partially working by scaling the viewport dimensions and centering the canvas with CSS to avoid the letterbox.

// Screen.ts
 private _computeFitContentScreen() {
    document.body.style.margin = '0px';
    document.body.style.overflow = 'hidden';
    this.canvas.style.position = 'absolute';
    
    const vw = window.innerWidth;
    const vh = window.innerHeight;

    const aspect = this.aspectRatio;
    let adjustedWidth = 0;
    let adjustedHeight = 0;
    if (window.innerWidth / aspect < window.innerHeight) {
      adjustedWidth = window.innerWidth;
      adjustedHeight = window.innerWidth / aspect;
    } else {
      adjustedWidth = window.innerHeight * aspect;
      adjustedHeight = window.innerHeight;
    }

    const scaleX = vw / adjustedWidth
    const scaleY = vh / adjustedHeight;

    const maxScaleFactor = Math.max(scaleX, scaleY);

    const zoomedWidth = adjustedWidth * maxScaleFactor;
    const zoomedHeight = adjustedHeight * maxScaleFactor;

    // Center zoomed dimension if bigger than the screen
    if (zoomedWidth > vw) {
      this.canvas.style.top = '';
      this.canvas.style.left = -(zoomedWidth - vw) / 2 + 'px';
    }

    if (zoomedHeight > vh) {
      this.canvas.style.top = -(zoomedHeight - vh) / 2 + 'px';
      this.canvas.style.left = '';
    }

    this.viewport = {
      width: zoomedWidth,
      height: zoomedHeight
    };
  }

  public get safeArea(): BoundingBox {
    if (this.displayMode === DisplayMode.ZoomToFitScreen) {
      // return safe area
      if (this.viewport.width > window.innerWidth) {
        const screenClip = (this.viewport.width - window.innerWidth) / this.viewport.width;
        const clip = screenClip * this.resolution.width;
        return new BoundingBox({
          top: 0,
          left: clip / 2,
          right: window.innerWidth - clip / 2,
          bottom: window.innerHeight
        });
      }

      if (this.viewport.height > window.innerHeight) {
        const screenClip = (this.viewport.height - window.innerHeight) / this.viewport.height;
        const clip = screenClip * this.resolution.height;
        return new BoundingBox({
          top: clip / 2,
          left: 0,
          right: window.innerWidth,
          bottom: window.innerHeight - clip / 2
        });
      }
    }
    return BoundingBox.fromDimension(this.viewport.width, this.viewport.height, Vector.Zero);
  }

If you're okay with it I can push these commits to your fork, let me know 👍

@eonarheim
Copy link
Member

I gave the docs an update, let me know if there are any gaps I should cover https://excaliburjs.com/docs/screens#hidpi-scaling

@eonarheim
Copy link
Member

eonarheim commented Mar 15, 2022

I think I have something that preserves aspect ratio and zooms to avoid black bar letterbox 👍

zoomtofit

@Joshua-Beatty
Copy link
Contributor Author

@eonarheim Is this demo you just posted the code you have above? If it is then I think we have each implemented the two best scaling modes from solar 2d. My code did what they call "letterbox" which keeps the content area always on screen, and from the demo it looks like you did "zoomEven", where it keeps the screen in the content, since it looks like when you resize vertically it does not change the scale(Unless you changed it to a vertical aspect ratio), where my code would scale down as the height decreased, bringing the sides in. Is that correct?

@eonarheim
Copy link
Member

@Joshua-Beatty indeed! The demo is roughly the code above (there were a few bugs I have ironed out locally) and appears to implement the "zoomEven" strategy from Solar2D. This demo code oversizes the viewport potentially losing some drawing area past the screen (or container) and keeps resolution and aspect ratio constant.

To address your question, I believe the original code in the fork keeps viewport constant to the container/screen and adjusts resolution, which seems similar to FillScreen/FillContainer. The current FillScreen and FillContainer modes dynamically adjust resolution and viewport to be the same fill up space in the container/screen (but doesn't keep aspect ratio constant). I think the difference is the fork uses the viewport aspect ratio as a reference (instead of resolution aspect ratio) to change resolution to match.

Theoretically the existing FitScreen display mode and FitContainer display mode implement the same as "letterbox" in Solar2D. These keep resolution and aspect ratio constant.

Is your desire to have something like "zoomEven" or "letterbox" in your game?

@Joshua-Beatty
Copy link
Contributor Author

Joshua-Beatty commented Mar 15, 2022

@eonarheim The letterbox mode in solar2d does one thing slightly different then FitScreen and FitContainer. While it does letter box it in, there are no "black bars" in solar 2d, instead that is just more canvas space to be used to display game objects etc. While in Excalibur it scales the aspect ratio to the size of the screen, but leaves the "black bars"(Or white bars by default, since it is just the body element). This is the display mode I prefer, and am attempting to implement in my fork, which I think I do successfully.

Fill screen and Fill container also do not work since the resolution of the screen in logical pixels change. which can be demonstrated how in my fork, if something is on the bottom left of the "content space" it will always be on screen, where in fill screen, it will be at the bottom left to start with, but then can be pushed off screen by resizing.

Edit: Though ZoomEven is also very useful, where for example you don't want someone looking too far left or right or too far up or down, where my letterbox implementation is useful when you always want someone to be able to see at least a specific amount up and down and at least a specific amount left and right.

So for my current project I'd like letterbox, but in the future I could use ZoomEven for another project.

@eonarheim
Copy link
Member

eonarheim commented Mar 15, 2022

@Joshua-Beatty I'm following now, thanks for sticking with me. I missed the extra usable canvas space portion of the "letterbox" Solar2D example.

Would we want to eventually define the safeArea to be the orange dotted line in that example to help with positioning content outside the main area?
image

@eonarheim
Copy link
Member

I think this PR will be a great addition to excalibur! I'll add the "zoomEven" things I have in a separate PR.

I propose we call these new display modes FitScreenAndFill and FitContainerAndFill.

We'll merge on tests, thanks again @Joshua-Beatty!

@Joshua-Beatty
Copy link
Contributor Author

@eonarheim I'll modify my fork with the new names. Are there any other changes you see? Other then adding tests?

Also, I know you have called it "safe area" but i have been playing around with some stuff and am thinking it should just be "content area". Because currently the content area could extend to the edge of the screen, even on an notched phone, and then actually calculating the true "safe area" could be difficult depending on weather someone is using cordova vs capacitor for example(Or if they're doing fill screen vs fill content), do you have any thoughts on how(and even if) safe area should be handled, or if it should be up to end user using like css env vars.

@eonarheim
Copy link
Member

eonarheim commented Mar 16, 2022

@Joshua-Beatty No other changes other than tests and new names needed to my eyes 👍

You make a good point about "safeArea" v. "contentArea", content area is probably more clear to a user as well. I'll need to think more about how we might handle fill (perhaps just an easy return with resolution as a bounding box?)

The notched screen might be difficult to know at all, unless there is some hint dropped in navigator or something...

Ope I just noticed the link you posted has a notch detect!

@eonarheim
Copy link
Member

HI @Joshua-Beatty Let me know if you need any assistance on the tests. Or if you would be okay with me adding commits to your fork.

@Joshua-Beatty
Copy link
Contributor Author

@eonarheim Absolutely feel free. I have been swamped with school, work, and other life stuff for a solid month now, and it will probably be a another few weeks before I actually have free time again.

@eonarheim eonarheim merged commit 41f251f into excaliburjs:main May 13, 2022
@eonarheim
Copy link
Member

@Joshua-Beatty Thanks again for working on this feature it will be an awesome addition, it'll be included in v0.26.0 very soon!!

eonarheim added a commit that referenced this pull request May 14, 2022
Related #2272

This PR implements a fit screen/container and zoom to avoid the letter boxing, this is achieved by over scaling the viewport and positioning. One caveat is this will clip some drawable area so should only be used in games that are okay with losing some draw space. This PR also adds a `ex.Screen.contentArea` which will return the "safe space" to place content given the current screen size.

New display modes `ex.DisplayMode.FitScreenAndZoom` and `ex.DisplayMode.FitContainerAndZoom`

![image](https://user-images.githubusercontent.com/612071/168433294-5bc3467a-f308-4e3f-9b78-30c0a3dce7b5.png)

Notice that some of the viewport is offscreen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FitContent DisplayModes
2 participants